feat(FR-2845): warn about cascading model card deletions when deleting associated vfolder#7339
Merged
Conversation
github-actions
Bot
added
area:ux
Contributor
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an explicit warning in the Admin Model Card delete flows when the user opts to also delete the associated model vfolder, clarifying that this can cascade into deleting model cards that depend on that folder.
Changes:
- Added a new i18n string for the cascade-deletion warning (currently in
enandko). - Updated the single-delete and bulk-delete confirmation modals to show an
Alertwhen “Also delete associated model folder(s)” is checked.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| resources/i18n/ko.json | Adds Korean translation for the new cascade-deletion warning string. |
| resources/i18n/en.json | Adds English translation for the new cascade-deletion warning string. |
| react/src/pages/AdminModelCardListPage.tsx | Shows a warning Alert in delete confirmation modals when associated vfolder deletion is enabled (single + bulk). |
ironAiken2
force-pushed
the
05-11-feat_fr-2845_warn_about_cascading_model_card_deletions_when_deleting_associated_vfolder
branch
from
06f8762 to
e7b4679
Compare
ironAiken2
force-pushed
the
05-11-feat_fr-2845_warn_about_cascading_model_card_deletions_when_deleting_associated_vfolder
branch
from
e7b4679 to
e5fd4e0
Compare
Merge activity
|
…g associated vfolder (#7339) Resolves #7308(FR-2845) ## Summary When deleting a model card with the **Also delete the associated model folder** option, the underlying vfolder is removed — and any other model cards that share that vfolder are silently deleted as well. The previous UI gave no indication of this cascade. This PR adds a strong inline warning to both the single-delete and bulk-delete model card modals. As soon as the user checks **Also delete the associated model folder**, an Alert appears below the checkbox: > Deleting the associated model folder will also delete every model card that uses it. The Alert intentionally uses `type="error"` for visual prominence, paired with the `ExclamationCircleFilled` warning icon — strong cue without misrepresenting the action as an actual error state. ## Changes - `react/src/pages/AdminModelCardListPage.tsx` — added conditional cascade-warning Alert under the "also delete folder" checkbox in both single and bulk delete modals. - `resources/i18n/{en,ko}.json` — added `adminModelCard.AlsoDeleteModelFolderCascadeWarning`. ## How to test 1. Open Admin > Model Cards. 2. Click the delete action on any model card whose vfolder is shared with other cards. 3. In the delete confirmation modal, check **Also delete the associated model folder** — the cascade warning Alert should appear immediately below the checkbox. 4. Repeat with a bulk delete (select multiple cards → Delete Selected). ## Notes - This is the simplified frontend response to FR-2845. The original ticket also suggested fetching `VFolder.modelCards` via the backend PR (`lablup/backend.ai#11480`) and listing the affected cards. That deeper integration is deferred — the inline warning communicates the cascade clearly without depending on a not-yet-merged backend change.
graphite-app
Bot
force-pushed
the
05-11-feat_fr-2845_warn_about_cascading_model_card_deletions_when_deleting_associated_vfolder
branch
from
e5fd4e0 to
6b352a7
Compare
graphite-app
Bot
deleted the
05-11-feat_fr-2845_warn_about_cascading_model_card_deletions_when_deleting_associated_vfolder
branch
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #7308(FR-2845)
Summary
When deleting a model card with the Also delete the associated model folder option, the underlying vfolder is removed — and any other model cards that share that vfolder are silently deleted as well. The previous UI gave no indication of this cascade.
This PR adds a strong inline warning to both the single-delete and bulk-delete model card modals. As soon as the user checks Also delete the associated model folder, an Alert appears below the checkbox:
The Alert intentionally uses
type="error"for visual prominence, paired with theExclamationCircleFilledwarning icon — strong cue without misrepresenting the action as an actual error state.Changes
react/src/pages/AdminModelCardListPage.tsx— added conditional cascade-warning Alert under the "also delete folder" checkbox in both single and bulk delete modals.resources/i18n/{en,ko}.json— addedadminModelCard.AlsoDeleteModelFolderCascadeWarning.How to test
Notes
VFolder.modelCardsvia the backend PR (lablup/backend.ai#11480) and listing the affected cards. That deeper integration is deferred — the inline warning communicates the cascade clearly without depending on a not-yet-merged backend change.